Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue #121] Fix broken embedded file references #138

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

Fragonite
Copy link
Contributor

@Fragonite Fragonite commented Nov 4, 2024

Summary

This pull request addresses the issue of broken links for customfield_textarea embedded files with a context ID of 1, which happens as part of pull request #131 and issue #121. The problem arises because text areas will now assume that any files are located in the course module context. This pull request updates the file table to resolve this issue.

Approach

New records are inserted with the course module context and updated filepathhash rather than updating the old file records. This approach has a few advantages:

  1. Backward Compatibility: Existing files fetched by system context will (should) continue to work.
  2. Cross-Referencing: New records can be cross-referenced with existing records.
  3. Future Cleanup: Old records can be cleaned up at a later date without affecting current functionality.

Testing Instructions

Testing this change is not straightforward but can be done by following these steps:

  1. Checkout the MOODLE_401_STABLE branch.
  2. Revert pull request #119.
  3. Edit an overview section and add some embedded files, such as images.
  4. Verify that these files use context ID 1.
  5. Unrevert pull request #119.
  6. Checkout pull request #131.
  7. Verify that the links are broken for the overview section.
  8. Checkout this branch and update your Moodle installation.
  9. Verify that the same overview section files now display correctly and can be backed up and restored without issue.

Related Issues

Additional Notes

I recommend backing up your database before applying these changes.
This should probably apply to more than just "overview" sections. What do you think?

@Fragonite Fragonite changed the base branch from main to issue121-401 November 4, 2024 15:08
db/upgrade.php Outdated
$sql = "SELECT f.*, ctx.id as ctxid
FROM {files} f
JOIN {customfield_data} cfd ON cfd.id = f.itemid
JOIN {customfield_field} cff ON cff.id = cfd.fieldid AND cff.shortname = 'overview'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue is for customfield which is using textarea, not only "overview".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, I have changed the sql to select cms types that use "fields" datasources.

@TomoTsuyuki
Copy link
Contributor

Please check CI error and fix it.

@Fragonite
Copy link
Contributor Author

Please check CI error and fix it.

CI is now green.

@TomoTsuyuki TomoTsuyuki merged commit dcddbce into issue121-401 Nov 6, 2024
11 checks passed
@TomoTsuyuki TomoTsuyuki deleted the issue121-401-files branch November 6, 2024 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants